Going through the QA, I hit an issue at Unsuccessful Post Import (Drop Resource) > Confirm that the dataset cedcd327-4e5d-43f9-8eb1-c11850fa7c55 has a Store value of waiting(0%) with an error in the Post Import column.

When I ran cron I got the following error:

$ ddev drush cron
 [notice] Localization of resource 3a187a87dc6cd47c48b6b4c4785224b7: 
 [notice] ImportService for 3a187a87dc6cd47c48b6b4c4785224b7__1735074165 completed.
 [error]  SQLSTATE[22007]: Invalid datetime format: 1292 Incorrect date value: '2049' for column 'objectid' at row 1 
 [error]  Failed to drop datastore for aba79c7a62fec6c2e49bc1cebd0d7233. No datastore storage found for 3a187a87dc6cd47c48b6b4c4785224b7:1735068191. 

On the dashboard, I do see the right error in the postImport column but the store says done:

I can confirm the datastore was not dropped and the objectId has reverted back to text (it was int after the previous QA steps):

GET https://kaise.ddev.site/api/1/datastore/query/cedcd327-4e5d-43f9-8eb1-c11850fa7c55/0?results=false&count=false

returns

{
  "schema": {
    "699890c9-480c-5f2e-89c5-06e9723303fb": {
      "fields": {
        "objectid": {
          "type": "text",
          "mysql_type": "text",
          "description": "OBJECTID"
        },
        "roadway": {
          "type": "text",
          "mysql_type": "text",
          "description": "ROADWAY"
        },
        "road_side": {
          "type": "text",
          "mysql_type": "text",
          "description": "ROAD_SIDE"
        },
        "lncd": {
          "type": "text",
          "mysql_type": "text",
          "description": "LNCD"
        },
        "descr": {
          "type": "text",
          "mysql_type": "text",
          "description": "DESCR"
        },
        "begin_post": {
          "type": "text",
          "mysql_type": "text",
          "description": "BEGIN_POST"
        },
        "end_post": {
          "type": "text",
          "mysql_type": "text",
          "description": "END_POST"
        },
        "shape_leng": {
          "type": "text",
          "mysql_type": "text",
          "description": "Shape_Leng"
        }
      }
    }
  },
  "query": {
    "results": false,
    "count": false,
    "resources": [
      {
        "id": "699890c9-480c-5f2e-89c5-06e9723303fb",
        "alias": "t"
      }
    ],
    "limit": 500,
    "offset": 0,
    "schema": true,
    "keys": true,
    "format": "json",
    "rowIds": false,
    "properties": [
      "objectid",
      "roadway",
      "road_side",
      "lncd",
      "descr",
      "begin_post",
      "end_post",
      "shape_leng"
    ]
  }
}

@dafeder Can you confirm you selected Drop the datastore table if the post import queue reports an error. on the Resource settings page (admin/dkan/resources) before attempting to run a failing post import?

I attempted the QA steps and was able to see "waiting (0%)" under the store column.

@dafeder I'm attempting to refactor so that I am satisfying this approach.

Make the entire storeResult() method on PostImportResult be essentially self-contained, so it contains the DB query and just needs to be passed a Connection object (ie the database service). Remove PostImport::storeJobStatus(). This would also remove Connection as a dependency for PostImport.

I would not be able to remove connection as a dependecy for PostImport because I would need to pass that in when instantiating PostImportResult. I will also need to add the connection service in the DashboardForm since I moved over all the db related code out of the PostImport class into the PostImportResult class.

@dafeder Can you confirm you selected Drop the datastore table if the post import queue reports an error. on the Resource settings page (admin/dkan/resources) before attempting to run a failing post import?

I attempted the QA steps and was able to see "waiting (0%)" under the store column.

I think so but possible I missed a step. I'll try again on Monday.

@dafeder I'm attempting to refactor so that I am satisfying this approach.

Make the entire storeResult() method on PostImportResult be essentially self-contained, so it contains the DB query and just needs to be passed a Connection object (ie the database service). Remove PostImport::storeJobStatus(). This would also remove Connection as a dependency for PostImport.

I would not be able to remove connection as a dependecy for PostImport because I would need to pass that in when instantiating PostImportResult. I will also need to add the connection service in the DashboardForm since I moved over all the db related code out of the PostImport class into the PostImportResult class.

You're right, apologies for not thinking about it more carefully. I had in my head there was a way to get the connection directly from DatastoreService but it looks like there is not. Disregard.

@dafeder I'm attempting to refactor so that I am satisfying this approach.

Make the entire storeResult() method on PostImportResult be essentially self-contained, so it contains the DB query and just needs to be passed a Connection object (ie the database service). Remove PostImport::storeJobStatus(). This would also remove Connection as a dependency for PostImport.

I would not be able to remove connection as a dependecy for PostImport because I would need to pass that in when instantiating PostImportResult. I will also need to add the connection service in the DashboardForm since I moved over all the db related code out of the PostImport class into the PostImportResult class.

I was able to use the database object like this $connection = \Drupal::database(); in the PostImportResult class. This way I can avoid injecting it into in the PostImport and DashboardForm classes. Pros and Cons doing it this way but we do satisfy code climate by reducing the number of arguments in the constructor.

Let's stick with the dependency injection pattern, the argument limit from CC is a good guideine but not an absolute rule.